Fix RSpec/ExampleWording in escaping and % string literals#1800
Fix RSpec/ExampleWording in escaping and % string literals#1800r7kamura wants to merge 1 commit into
RSpec/ExampleWording in escaping and % string literals#1800Conversation
|
This cop has not been able to handle |
| end | ||
|
|
||
| def needs_escape?(node) | ||
| node.value.include?(node.loc.begin.source) || |
There was a problem hiding this comment.
For quoted strings, both begin and end are the same. Do we need to check twice?
There was a problem hiding this comment.
This is for %q(...), %Q(...), or %<...> etc.
irb(#<RuboCop::Cop::RSpec::Exampl...):001> node.loc.begin.source
=> "%q("
irb(#<RuboCop::Cop::RSpec::Exampl...):002> node.loc.end.source
=> ")"
But on second thought, we must check against node.loc.begin.source[-1], not node.loc.begin.source. I'll try to fix that.
There was a problem hiding this comment.
hmm, since escaping should only be necessary if begin and end are different (e.g. %!...! may require escaping like \!, while %(...) does not), it seems that only one of them needs to be checked after all.
pirj
left a comment
pirj
left a comment
There was a problem hiding this comment.
Thank you!
I’d love to extensively test this, but only the quoting autocorrection part. Wondering how to do that.
r7kamura
force-pushed
the
example-wording-escape
branch
2 times, most recently
from
6b470ba to
ffc6a8c
Compare
r7kamura
changed the title
RSpec/ExampleWording autocorrection to correctly escape quotes on str node caseRSpec/ExampleWording in escaping and % string literals
|
I've added a few more improvements:
Since the handling of |
r7kamura
force-pushed
the
example-wording-escape
branch
5 times, most recently
from
35ffb03 to
2ef3761
Compare
| RUBY | ||
| end | ||
|
|
||
| context "when message includes `'` in `'...'`" do |
There was a problem hiding this comment.
I think these examples are in the wrong context ('when DisallowedExamples: Workz').
| RUBY | ||
|
|
||
| expect_correction(<<~RUBY) | ||
| it "returns foo's bar" do |
There was a problem hiding this comment.
Maybe it’s a silly question, but why are we changing the type of quotes from ' to " here? If we kept the same quote type, we wouldn’t need to concern ourselves with escaping, would we? (unless we insert an apostrophe/single quote)
There was a problem hiding this comment.
This is for implementation convenience. If there is an implementation like string.inspect(quote_type: 'single_quote'), I would like to use it. If there is a way to rewrite only substrings while preserving quoting and escaping, that would be great, but do you have any other ideas?
There was a problem hiding this comment.
This is a just idea, but, if the offense range had been like this, we might not have to go through all this trouble?
it "should return foo's bar"
^^^^^^^^^^^^^
then we would only need to replace this range without changing the unrelated part that may contain troublesome
escapes.
…on str node case
r7kamura
force-pushed
the
example-wording-escape
branch
from
2ef3761 to
d860600
Compare
4 participants
This change resolves some of the following Issue:
RSpec/ExampleWordingunintentionally removes escapes #1799It still does not handle quotes well for dstr node, but at least it will handle them well for str node.
Before submitting the PR make sure the following are checked:
master(if not - rebase it).CHANGELOG.mdif the new code introduces user-observable changes.bundle exec rake) passes (be sure to run this locally, since it may produce updated documentation that you will need to commit).If you have created a new cop:
config/default.yml.Enabled: pendinginconfig/default.yml.Enabled: truein.rubocop.yml.VersionAdded: "<<next>>"indefault/config.yml.If you have modified an existing cop's configuration options:
VersionChanged: "<<next>>"inconfig/default.yml.